CBFS size on T420 and X220 is already at absolute maximum (pending PR).

@SebastianMcMillan #693 you mean? I'll rebase on top of that once merged

@SebastianMcMillan all good now I think

This PR is the followup of #709?

@MrChromebox @SebastianMcMillan : 4.12 is populating exactly the same as here?

Will try to test in the next days on x230.

This PR is the followup of #721

bit of a circular reference :) Assume you meant a different PR?

@MrChromebox corrected past reference. Questioning it now.

Well I do not know if it was expected to work but from my results, neither of the following are populated PCRs supposedly measured by Heads:

0: Boot block
1: ROM stage
3: Heads Linux kernel and initrd
4: Boot mode (0 during /init, then recovery or normal-boot)
5: Heads Linux kernel modules

While PCRs 0-3 are supposed to be filled by coreboot and do not represent what Heads is supposed to measure by its coreboot patch:

0: Heads: Boot block. Coreboot: Google vboot GBB flags.
1: Heads: ROM stage. Coreboot: Google vboot GBB HWID
2: Heads: RAM stage. Coreboot: Core Root of Trust for Measurement which includes all stages, data and blobs.
3: Heads: Linux kernel and initrd.  Coreboot: Runtime data like hwinfo.hex or MRC cache.

While PCRs 6-7 of Heads, not conflicting by coreboot, are still measured here per screenshot:

6: Drive LUKS headers
7: Heads user-specific config files

Heads measurements doc
Coreboot measurement docs

@MrChromebox only done functional testing here, setting a default boot option and mystyping disk unlock key passphrase to have pcrs called from /etc/functions. I do not know a functional working testing path outside of this since going to the recovery invalidates the measurements by extending them, while going forward just trust measurements to be valid. This is not the case here.

Was it thought to be functional and a simple repackaging of #709?
@PatrickRudolph?

Screenshot:

@MrChromebox :
I see patches removed to coreboot but none added from #709, so no measured boot because needed patch not in, or core logic of heads needing to change? @PatrickRudolph?

@tlaurion I don't see any coreboot-related patches in #709, with the exception of the SMM patch, which is already merged in coreboot 4.12.

And with 4.12, measured boot is decoupled from vboot, so vboot isn't necessary (though it would certainly make sense to rebase #709 to add vboot in addition to measured boot).

@MrChromebox corrected past reference. Questining it now.

Well I do not know if it was expected to work but from my results, neither of the following are populated PCRs:

0: Boot block
1: ROM stage
3: Heads Linux kernel and initrd
4: Boot mode (0 during /init, then recovery or normal-boot)
5: Heads Linux kernel modules

Ref

@MrChromebox only done functional testing here, setting a default boot option and mystyping disk unlock key passphrase to have pcrs called from /etc/functions. I do not know a functional working testing path outside of this since going to the recovery invalidates the measurements by extending them, while going forward just trust measurements to be valid. This is not the case here.

Was it thought to be functional and a simple repackaging of #709?
@PatrickRudolph?

Screenshot:

The PCRs are extended like it's documented here: https://doc.coreboot.org/security/vboot/measured_boot.html
That's not how heads extens the PCRs, so you need another patch on top of 4.12 to keep the current behaviour.

@MrChromebox : i'm not sure everybody will agree here, but since patches are applied to versions and commit ids now, I do not think removing patches is a good idea.

coreboot 4.8.1 patches could stay in repo forever.
coreboot 4.11 patches, 4.12 patches...

We could force boards to use specific versions of coreboot (kgpe-d16) instead of deleting boards.

@PatrickRudolph @MrChromebox : I'm not sure what are the next steps here.
@MrChromebox : 4.11 patches for vboot+measured boot are here

@tlaurion are you suggesting we keep older versions of modules and patches in tree and buildable? that's a nightmare to maintain w/r/t build tools etc. If anything, branch off for older devices and they can be maintained by interested parties, but let's not carry everything forward forever

The coreboot 4.11+vboot PR has the same issue with PCRs being used differently by coreboot.

I'll take a look at coreboot 4.11+'s measured boot and see if I can extend the PCRs to match the previous implementation, but won't have time to do that until next week

@MrChromebox I was talking about a way to say:

• Coreboot version in board config (4.8.1, 4.11, 4.12)
• keeping multiple coreboot modules with version appended (module/coreboot-4.8.1)
• keeping multiple coreboot config consequentially
• keeping multiple coreboot specific patch directories/files

This way, someone wanting to build kgpe-d16 could still make BOARD=kgpe-d16 in CI when #571 goes forward without having to branch. Heads would continue to support boards, while if support is brought back by patches or newer coreboot version, boards could simply specify newer coreboot version.

Limitation:

• Patches are applied at module package decompression. So adding a new patch requires to delete build/module_ver else patch is not applied.

The outcome of this would be to be able to produce reproducible board roms as artifacts. As of today, the x230-hotp-board is build from CircleCI (debian) and GitlabCI (Fedora).

@tlaurion but it's not just keeping coreboot 4.8.1 and patches, it's maintaining the toolchain, and likely a fixed version of flashrom and the kernel as well. If we decide we want to fix those for all boards, then there's an exponential increase in work to maintain, and an inertia to keep boards on older versions if they work there.

I feel like the way coreboot does things with periodic versioned releases, and dropping non-compliant boards after a new release is the only sane way to do things. Tag/branch a release before we move to coreboot 4.12, and let legacy boards live there.

What I propose is to keep/modify:

• Board specifying coreboot + version (EDIT: see coreboot: 4.8.1 fixed in Makefile, coreboot module and board configs #776 )
• module/coreboot_version
• keep patches just like they are right now, being applied for commit id/version

Limitations:

• Patches are applied at module's package decompression only. So adding new patches would require CI's cache clearance and/or manual build/coreboot-* removal if the patches list is different.
• This approach would permit old coreboot versions to be maintained with backported patches for older boards when required.

Effect:

• kgpe-d16 board and other boards that will be dropped by coreboot could still be built by CIs, while other boards still maintained would require board version switch upon being tested functional to change versioning to newer version.
  • I do not think other modules require the same principle, while the implementated logic for coreboot could probably be applied to linux module, so the danger of breaking a board would be limited, while augmenting a tiny bit the maintainership needed per contributors to test module upgrades. The good side affect of doing this is that boards that are working now would continue to work, while maintained boards could benefit from new features and new modules.

@MrChromebox @SebastianMcMillan @merge : thoughts?

my main concern is with the toolchain, and changes to it now affecting 2 (or more) versions of coreboot

Hi. Compiling of this completes succesfull, but flashing the rom gives me no video output on my x230 with modded fhd display even if I swap the data.vbt and gma-mainboard.ads in the coreboot files with their patched equivalents. It used to work with the merge:coreboot_next patch. Any ideas what could cause this? How can I get this to work on the x230 with fhd mod?

@AdmerStroh does coreboot 4.12 + SeaBIOS/Tianocore work properly with the FHD modifications? I don't know what changes might have affected the FHD mod between 4.11 and 4.12, but I'm guessing something on the libgfxinit side. What's the modded gma-mainboard file look like?

@MrChromebox It does (the latest version of Skulls works just fine without any additional modifications). The modded gma-mainboard file has just LVDS output removed (see https://review.coreboot.org/c/coreboot/+/28950/10/src/mainboard/lenovo/x230/variants/x230_fhd/gma-mainboard.ads). It is strange regarding that stock coreboot 4.12 works just fine.. Maybe someone with a fhd-modded x230 could test if it works for them?..

@AdmerStroh is the screen backlight coming on? the default config for the x230 w/coreboot 4.12 under Heads should leave libgfxinit as the default display init. Unfortunately I only have a standard x230 here to test, not a FHD one

@MrChromebox The backlight is coming on and can be dimmed via press on the power button. I will try to force output over the mini-dp port and get something on an external monitor. I will report the results.

@MrChromebox unfortunately, even with only HDMI1 in gma-mainboard file enabled, no video output over external monitor (mini-dp). Maybe I made a fault merging your patch? For now, I will just use skulls or revert back to 4.11. Maybe someone more experienced than me with a x230 fhd mod could try and share their experience. Nonetheless thank you!

@MrChromebox unfortunately, even with only HDMI1 in gma-mainboard file enabled, no video output over external monitor (mini-dp). Maybe I made a fault merging your patch? For now, I will just use skulls or revert back to 4.11. Maybe someone more experienced than me with a x230 fhd mod could try and share their experience. Nonetheless thank you!

@AdmerStroh : my intuition here may be how you cleaned me.
Have you followed https://github.com/osresearch/heads-wiki/blob/master/Clean-the-ME-firmware.md ?

Edit: maybe not, since this seems valid even if ME wasn't cleaned completely: https://github.com/osresearch/heads/pull/721/files#diff-2d4ed2860b55bb308eacc61e6c18a86fR5

@tlaurion I used the skulls script with the me-cleaning option on the bottom spi chip. Will it be reinstalled when I internally flash the full 12mb rom image? By the way, compiling and externally flashing the 4MB x230-flash rom with this patch works and I get a console output

@AdmerStroh @MrChromebox this is why I'm wondering where your absence of console comes from on full rom image (230's coreboot.rom) since x230-flash.rom image gives console output.

@tlaurion @MrChromebox Ok. I could succesfully resolve my issue by applying this patch #703 and flashing both the top and the bottom image externally. So probably tlaurion was right with the me not properly cleaned in the bottom chip.. Thank you very much for the help :)

@tlaurion @MrChromebox Ok. I could succesfully resolve my issue by applying this patch #703 and flashing both the top and the bottom image externally. So probably tlaurion was right with the me not properly cleaned in the bottom chip.. Thank you very much for the help :)

@AdmerStroh : please make sure you read the big fat warning I just added here

If you intend to flash through x230-flash image, you are not supposed to play with #703 at all.

#703 is WiP and is not ready, hence why it is not merged in master.
#703 results in a board that flashes the whole 12 SPI, considering that ME was fully neutered and all space was freed; images internally flashed are wholesome. This is not the case with x230 images, that leaves IFD unfreed space unused and only flash the BIOS region of SPI...

If you come from x230-flash or Skulls, you need to apply x230 image from within x230-flash, and stay with x230 board upgrades.

Images built from master can be downloaded from artifacts here and here.

Note that as you can see, the builds are not yet reproducible since this issue is not yet resolved

28d3b7c being merged, #721 #709 are next.
@MrChromebox @PatrickRudolph

@MrChromebox ok. So logic is inverted and fallsback to 4.12 default for boards not specifying other version.

Testing with x230-htop-verification board removing coreboot 4.8.1 version here: https://app.circleci.com/pipelines/github/tlaurion/heads/292/workflows/8cb5121b-dc5d-4099-8aed-ad80cd1cdea0/jobs/317

@MrChromebox : not specifying the coreboot version under test board x230-hotp-verification resulted in this error:

make[1]: Entering directory '/root/project'
modules/coreboot:10: *** "x230-hotp-verification: does not specify coreboot version under CONFIG_COREBOOT_VERSION".  Stop.
make[1]: Leaving directory '/root/project'
make: *** [Makefile:587: all] Error 2

Src: https://app.circleci.com/pipelines/github/tlaurion/heads/292/workflows/8cb5121b-dc5d-4099-8aed-ad80cd1cdea0/jobs/317/parallel-runs/0/steps/0-123

Doing manual tests locally.
Edit: Misunderstood that 4.12 needed to be specified in board configs. Doing.

@tlaurion yeah was about to say, the behavior w/r/t specifying the coreboot version is unchanged. No logic inverted, no fallback

Ok, CI system up and running for x230 x230-hotp-verification, t430, qemu-coreboot here with artifacts:https://app.circleci.com/pipelines/github/tlaurion/heads/298/workflows/795e66c6-4ad4-4c1d-bf08-c439330124dd/jobs/324

t430: ( @flawedworld )
t430-flash: https://324-103208611-gh.circle-artifacts.com/0/build/t430-flash/coreboot.rom
t430: https://324-103208611-gh.circle-artifacts.com/0/build/t430/coreboot.rom

x230: (@tlaurion @osresearch @merge @jan23 @flawedworld @MrChromebox @shamen123 @eganonoa)
x230-flash: https://324-103208611-gh.circle-artifacts.com/0/build/x230-flash/coreboot.rom
x230: https://324-103208611-gh.circle-artifacts.com/0/build/x230/coreboot.rom
x230-hotp-verification: https://324-103208611-gh.circle-artifacts.com/0/build/x230-hotp-verification/coreboot.rom

qemu-coreboot: https://324-103208611-gh.circle-artifacts.com/0/build/qemu-coreboot/coreboot.rom

I had to patch this PR. @MrChromebox Please review
patch.txt or https://github.com/tlaurion/heads/tree/x230-hotp-verification_coreboot412

@tlaurion I don't see why you need to patch anything outside of the board/coreboot configs, unless your fork isn't up to date. The 'libremkey-hotp-verification' module doesn't exist anymore so that tells me it isn't.

Any reason the Librem boards (which need to stay on 4.8.1 for now) aren't being built (without blobs) as part of CI testing?

@PatrickRudolph : unfortunately, the x230-hotp-verification build here (and I suspect all other xx30 boards) still suffer from #770. As said in other tickets, that regression came out after changing the buildchain from musl-cross to musl-cross-make, since https://gitlab.com/tlaurion/heads/-/jobs/312958349/artifacts/raw/build/x230-libremkey/coreboot.rom (same coreboot 4.8.1, same QubesOS) is able to resume from suspend. Would need help here.

Interestingly enough, #765 is gone. Any idea? Rest is feature pair in terms of measurements and basic testing works as a direct replacement.

Personally, as for @jans23 and the general community, #770 is a show stopper.

also, even if issues migrating boards to 4.12, we should still go ahead and merge this, since no impact to existing boards and it will allow adding of other new boards (like the Librem Mini) which require 4.12

I don't see why #770 would block this PR.

@tlaurion I don't see why you need to patch anything outside of the board/coreboot configs, unless your fork isn't up to date. The 'libremkey-hotp-verification' module doesn't exist anymore so that tells me it isn't.

@MrChromebox rebased my branch over osresearch/master just to make sure. Here is comparison over this branch
https://github.com/MrChromebox/heads/compare/coreboot-4.12...tlaurion:x230-hotp-verification_coreboot412?expand=1

Any reason the Librem boards (which need to stay on 4.8.1 for now) aren't being built (without blobs) as part of CI testing?

Just because I didn't added them yet and thought that blobs needed to be present as specified into blobs dir and extracted from full roms, just like the xx20 boards. With #798, adding those boards into CIs configs will not add much build time. Want to do provide a PR? Please put boards in blocks with similarities of coreboot+linux near of each other in CircleCI and GitlabCI configs.

@MrChromebox added missing boards in latest commit MrChromebox@d643838

Building here: https://app.circleci.com/pipelines/github/tlaurion/heads/301/workflows/086cf9e6-d2d4-4892-9b43-225e927eb563/jobs/327

@MrChromebox

Any reason the Librem boards (which need to stay on 4.8.1 for now) aren't being built (without blobs) as part of CI testing?

But the build will obviously fail because blobs are not provided in tree:
https://github.com/osresearch/heads/blob/master/config/coreboot-librem13v2.config#L13-L23
https://github.com/osresearch/heads/blob/master/config/coreboot-librem13v4.config#L13-L23
https://github.com/osresearch/heads/blob/master/config/coreboot-librem15v3.config#L13-L23
https://github.com/osresearch/heads/blob/master/config/coreboot-librem15v4.config#L13-L23

So I will remove those boards in next commit once that point is proven with current board+coreboot configs, which is why the x220 and t420 (which have blobs/* corresponding directories) are not built from CIs since not blob free as of now.
https://github.com/osresearch/heads/blob/master/config/coreboot-x220.config#L13-L18
https://github.com/osresearch/heads/blob/master/config/coreboot-t420.config#L13-L18

You can join forces #307, that ticket would feel less lonely.

@tlaurion comparing branches isn't helpful when commits get merged out of order (changing the hashes) and there is no unique ID associated with a given commit. We should think about adding one as part of a commit hook like coreboot does.

I still maintain that this PR is completely sufficient as-is. It touches a single file and only adds the option to build against coreboot 4.12. Building boards using it and the changes needed there should be handed in separate PRs and not affect this one.

edit: I've rebased this patch on master to avoid any confusion, though it shouldn't have been required

@MrChromebox agreed. This needs to be changed and it builds for all current boards.

I'll do a seperate PR for CI changes and coreboot CBFS change needed for x230 to build this was simply to provide roms for people to test prior of merging.

@MrChromebox agreed. This needs to be changed and it builds for all current boards.

well, as part of migrating boards to 4.12, we should update the defconfigs as well -- increasing the CBFS size as needed, dropping config items that make no sense, etc